fix(discovery-index): treat body-signalled secondary rate limits as retryable by bitfathers94 · Pull Request #10249 · JSONbored/loopover

bitfathers94 · 2026-07-31T14:19:24Z

fix(discovery-index): treat body-signalled secondary rate limits as retryable

isRateLimitStatus only inspected status/retry-after/x-ratelimit-remaining, so
a GitHub secondary rate limit (403 with body-only signalling, no retry-after,
remaining still non-zero) fell through as a permanent failure and silently
truncated the fan-out's candidate set. Port the body-regex check from
src/github/client.ts's isRateLimitedResponse, reading via clone().text() so
callers can still read the response body on the retried-and-succeeded path.

Closes #10030

…etryable isRateLimitStatus only inspected status/retry-after/x-ratelimit-remaining, so a GitHub secondary rate limit (403 with body-only signalling, no retry-after, remaining still non-zero) fell through as a permanent failure and silently truncated the fan-out's candidate set. Port the body-regex check from src/github/client.ts's isRateLimitedResponse, reading via clone().text() so callers can still read the response body on the retried-and-succeeded path.

loopover-orb · 2026-07-31T14:19:33Z

Tip

✅ LoopOver review result - approve/merge recommended

_{Review updated: 2026-07-31 14:25:59 UTC}

2 files · 1 AI reviewer · no blockers · readiness 93/100 · CI green · clean

✅ Suggested Action - Approve/Merge

safe to merge

Review summary
The PR converts isRateLimitStatus/isRetryableStatus to async so a cloned response body can be regex-checked for GitHub's secondary-rate-limit/abuse-detection phrasing, mirroring the existing check in src/github/client.ts, and updates the single call site to await it. The change is correctly threaded through fetchWithRetry (the only caller), uses response.clone() so the body remains readable by downstream json() calls, and is covered by new tests for both wording variants, an unreadable-clone fallback, and a regression test for the truncated-fan-out scenario described in the issue.

Nits — 5 non-blocking

packages/discovery-index/src/github-client.ts:76 — the bare regex `/secondary rate limit|\babuse\b|api rate limit exceeded/i` is inlined; consider extracting it as a named constant (as the brief notes) so the two GitHub client implementations (src/github/client.ts and this one) can be kept in sync more easily, and to self-document why these three phrases are checked.
test/unit/discovery-index/github-client.test.ts — the four new tests are a good, non-fabricated set (they exercise the real fetchWithRetry path), but there's no test for a 403 with a body that matches none of the three phrases and no rate-limit headers, i.e. the plain-403 fallthrough combined with a readable-but-irrelevant body; worth adding for full branch coverage of the new regex line.
packages/discovery-index/src/github-client.ts:64-70 — the try/catch around `response.clone().text()` is marked `v8 ignore` as unreachable in practice, which is a reasonable call given clone() on a real fetch Response should not fail, but it's still worth a one-line comment on *why* the test at line 250 uses a hand-rolled fake Response rather than a real one (since real Response objects can't easily be made to reject `.text()`).
Extract the regex in github-client.ts:64 into a shared constant or module (ideally reused from src/github/client.ts's isRateLimitedResponse) to avoid the two implementations drifting apart if GitHub adds new wording.
Add a test case for a 403 whose body is readable but doesn't match any of the three phrases, to lock in the non-retry fallthrough now that the function has a new async branch.

Decision drivers

✅ Code review — No blockers (1 reviewer)
✅ Gate result — Passing (No configured blocker found.)

Context & advisory signals — never blocks the verdict

Signal	Result	Evidence
Linked issue	✅ Linked	#10030
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (1 linked issue).
Validation posture	⚠️ 20/25	Preflight is ready, but the PR body does not name the validation run.
Contributor workload	✅ 10/10	Author activity: 140 registered-repo PR(s), 99 merged, 3 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	bitfathers94; Gittensor profile; 140 PR(s), 3 issue(s).
Improvement	✅ Minor	risk: clean · value: minor · LLM: moderate

Linked issue satisfaction

Addressed
The diff makes isRateLimitStatus/isRetryableStatus async and adds the same clone().text() regex body check as src/github/client.ts, with try/catch degrading to false, and fetchWithRetry awaits the async check while leaving recordRateLimit/backoff/metrics untouched; tests cover the required secondary-limit retry, unreadable-clone fallback, and non-truncation regression scenarios.

Review context

Author: bitfathers94
Role context: outside_contributor
Public audience mode: oss maintainer
Lane context: Repository is configured for direct PR review.
Public profile languages: TypeScript
Official Gittensor activity: 140 PR(s), 3 issue(s).
PR-specific overlap: none found.

Contributor next steps

Start here: Add validation command/output.

Signal definitions

Related work = same linked issue, overlapping active PRs, or title/path similarity.
Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

@loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
@loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

Re-run LoopOver review

superagent-security · 2026-07-31T14:20:20Z

Superagent didn't find any vulnerabilities or security issues in this PR.

codecov · 2026-07-31T14:25:21Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.61%. Comparing base (78891b8) to head (d6e5d8a).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff            @@
##             main   #10249    +/-   ##
========================================
  Coverage   80.60%   80.61%            
========================================
  Files         283      284     +1     
  Lines       59101    59210   +109     
  Branches     7000     7033    +33     
========================================
+ Hits        47641    47730    +89     
- Misses      11167    11188    +21     
+ Partials      293      292     -1

Flag	Coverage Δ
backend	`100.00% <100.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
packages/discovery-index/src/github-client.ts	`100.00% <100.00%> (ø)`

... and 1 file with indirect coverage changes

loopover-orb

LoopOver approves — the gate is satisfied and CI is green.

bitfathers94 requested a review from JSONbored as a code owner July 31, 2026 14:19

loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 31, 2026

loopover-orb Bot assigned bitfathers94 Jul 31, 2026

loopover-orb Bot approved these changes Jul 31, 2026

View reviewed changes

loopover-orb Bot merged commit 9a84fcd into JSONbored:main Jul 31, 2026
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(discovery-index): treat body-signalled secondary rate limits as retryable - #10249

fix(discovery-index): treat body-signalled secondary rate limits as retryable#10249
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
bitfathers94:fix/issue-10030

bitfathers94 commented Jul 31, 2026

Uh oh!

loopover-orb Bot commented Jul 31, 2026 •

edited

Loading

✅ LoopOver review result - approve/merge recommended

Uh oh!

superagent-security Bot commented Jul 31, 2026

Uh oh!

codecov Bot commented Jul 31, 2026

Uh oh!

loopover-orb Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

bitfathers94 commented Jul 31, 2026

Uh oh!

loopover-orb Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ LoopOver review result - approve/merge recommended

Uh oh!

superagent-security Bot commented Jul 31, 2026

Uh oh!

codecov Bot commented Jul 31, 2026

Codecov Report

Uh oh!

loopover-orb Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

loopover-orb Bot commented Jul 31, 2026 •

edited

Loading